Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pre/postEventFunction to addDebugHook #126

Merged

Conversation

Necktrox
Copy link
Contributor

@Necktrox Necktrox commented May 1, 2017

This PR adds, see title, two new debug hook types called preEventFunction and postEventFunction. You can now have greater control over event handling throughout your entire server if you want to debug your own implemented functionality in this region.

Wiki: addDebugHook

string preEventFunction( resource eventResource, string eventName, element eventSource, element eventClient, string eventFilename, int eventLineNumber, resource functionResource, string functionFilename, int functionLineNumber, ...eventArgs )
       postEventFunction( resource eventResource, string eventName, element eventSource, element eventClient, string eventFilename, int eventLineNumber, resource functionResource, string functionFilename, int functionLineNumber, ...eventArgs )

@qaisjp qaisjp added the enhancement New feature or request label May 6, 2017
Copy link
Contributor

@qaisjp qaisjp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice stuff. I've suggested a thing or two, but if you've given this thorough testing, feel free to adjust the labels and give this a merge 🚢 🇮🇹 (:shipit:)

@@ -132,7 +136,7 @@ bool CDebugHookManager::RemoveDebugHook( EDebugHookType hookType, const CLuaFunc
///////////////////////////////////////////////////////////////
void CDebugHookManager::OnLuaMainDestroy( CLuaMain* pLuaMain )
{
for( uint hookType = EDebugHook::PRE_EVENT ; hookType <= EDebugHook::POST_FUNCTION ; hookType++ )
for( uint hookType = EDebugHook::PRE_EVENT ; hookType <= EDebugHook::POST_EVENT_FUNCTION ; hookType++ )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could DUMMY_LAST_DEBUG_HOOK be used here, and defined where the enums are defined?

@@ -193,6 +193,9 @@ bool CMapEventManager::Call ( const char* szName, const CLuaArguments& Arguments
if ( bEnabled )
g_pCore->LogEvent ( 0, "Lua Event", pMapEvent->GetVM ()->GetScriptName (), szName );

if ( !g_pClientGame->GetDebugHookManager()->OnPreEventFunction( szName, Arguments, pSource, NULL, pMapEvent ) )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is nullptr possible?

@Necktrox Necktrox force-pushed the feature/event-function-debughook branch from ae75dd8 to 3c046f8 Compare July 9, 2017 23:57
@Necktrox
Copy link
Contributor Author

I don't have any usecase for this anymore personally. Does anybody need this? If not, then there is no reason to merge this.

@qaisjp
Copy link
Contributor

qaisjp commented Feb 15, 2018

What was your use-case?

@Necktrox
Copy link
Contributor Author

To measure CPU usage for onClientRender per resource/eventhandler, which you can't do with onPreEvent onPostEvent.

@AlexTMjugador
Copy link
Member

AlexTMjugador commented Feb 16, 2018

I feel that giving scripters and server owners profiling tools like this can't hurt, neither be a bloat, but indeed it can be something useful to gather information about how are concrete parts of a resource behaving.

Talking about the particular use-case that @Necktrox mentioned, I think that onClientRender can be a major performance bottleneck for the server when not so well designed algorithms are in place or just when a need to do extremely frequently operations arises (depending on the gamemode, it may be necessary to tie some logic at the execution speed of the game. And it could happen that one big resource does those operations). One could argue that in well designed scripts that shouldn't be a performance hog, and it would be right. But at least this PR would provide people who design that performance hogs tools to prove that they are doing something wrong, right?

Besides, CPU usage can be a concern for server owners (albeit unlikely sor a MTA server alone), so anything that could give them more factual data about how are the CPU cycles being used seems welcome.

@Necktrox Necktrox merged commit 9eef046 into multitheftauto:master Feb 18, 2018
@Necktrox
Copy link
Contributor Author

Necktrox commented Feb 18, 2018

What's possible:
pre/postEventFunction
pre/postEvent
pre/postFunction

Same examples, but hosted on GitHub Gist:
pre/postEventFunction
pre/postEvent
pre/postFunction

@patrikjuvonen patrikjuvonen added this to In progress in release/v1.5.6 via automation Aug 7, 2018
@patrikjuvonen patrikjuvonen added this to the 1.5.6 milestone Aug 7, 2018
@patrikjuvonen patrikjuvonen moved this from In progress to Done in release/v1.5.6 Aug 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants